Skip to content

[Enhancement] Optimized fast_path_match()#86

Merged
kezhenxu94 merged 1 commit into
apache:masterfrom
tom-pytel:master
Nov 22, 2020
Merged

[Enhancement] Optimized fast_path_match()#86
kezhenxu94 merged 1 commit into
apache:masterfrom
tom-pytel:master

Conversation

@tom-pytel

@tom-pytel tom-pytel commented Nov 22, 2020

Copy link
Copy Markdown
Contributor
  • Changed fast_path_match() to use Python regex instead of working one character at a time.
  • Also pulled out invariant disable_patterns out of loop in plugins.install().

@kezhenxu94 kezhenxu94 added the enhancement New feature or request label Nov 22, 2020
kezhenxu94
kezhenxu94 reviewed Nov 22, 2020
View reviewed changes
Comment thread skywalking/utils/ant_matcher.py
#


def fast_path_match(pattern: str, path: str):

@kezhenxu94 kezhenxu94 Nov 22, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was ported from a Java version, I've just run a simple benchmark test, regex is much more better then the original version, thanks

kezhenxu94
kezhenxu94 approved these changes Nov 22, 2020
View reviewed changes

@kezhenxu94 kezhenxu94 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kezhenxu94 kezhenxu94 added this to the 0.5.0 milestone Nov 22, 2020
@kezhenxu94 kezhenxu94 merged commit 5e42df2 into apache:master Nov 22, 2020
@tom-pytel

tom-pytel commented Nov 22, 2020

Copy link
Copy Markdown
Contributor Author

Hey, can't seem to find a way to open an issue on the NodeJS agent so will ask here, having a problem running the test due to the following:

Step 1/5 : FROM nodejs-dependencies
    ✓ test /home/tom/src/revdebugjs/skywalking-nodejs/tests/plugins/http
Service 'server' failed to build: pull access denied for nodejs-dependencies, repository does not exist or may require 'docker login': denied: requested access to the resource is denied

Any idea?

@wu-sheng

wu-sheng commented Nov 22, 2020

Copy link
Copy Markdown
Member

Hi @tom-pytel we share the issue list in the main repo to make people easier, feel free to submit a discussion here, https://github.com/apache/skywalking/issues with project name as the prefix of the issue title. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kezhenxu94 kezhenxu94 kezhenxu94 approved these changes

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

0.5.0

Development

Successfully merging this pull request may close these issues.

3 participants

@tom-pytel @wu-sheng @kezhenxu94